feat: Add kubectl plugin support and enhance EKS aperf script #323
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| --namespace) dest="NAMESPACE";; | ||
| --aperf_options) dest="APERF_OPTIONS";; | ||
| --aperf_image) dest="APERF_IMAGE";; | ||
| --cpu-request) dest="CPU_REQUEST";; |
There was a problem hiding this comment.
Any reason why you remove such options?
|
|
||
| # Get node taints and generate tolerations | ||
| echo -e "${BOLD}Checking node taints...${NC}" | ||
| TAINTS=$(kubectl get node ${NODE_NAME} -o jsonpath='{.spec.taints[*]}' 2>/dev/null) |
There was a problem hiding this comment.
we probably need to add a check if the NODE exists before check the taint, otherwise if user enter a wrong node, it fails without a clear reason. What do you think?
| if [[ "$OSTYPE" == "darwin"* ]]; then | ||
| # macOS | ||
| open "$INDEX_FILE" | ||
| elif [[ "$OSTYPE" == "linux-gnu"* ]]; then |
There was a problem hiding this comment.
Windows support?
| POD_NAME="aperf-pod-${NODE_NAME//[.]/-}" | ||
|
|
||
| # Get node taints and generate tolerations | ||
| echo -e "${BOLD}Checking node taints...${NC}" |
There was a problem hiding this comment.
This goes to a new line, can we have the result on a single line?
bash ./kubectl-aperf --aperf_image="${APERF_ECRREPO}:latest" --node="i-087ae37512508cca5"
Checking node taints...
No taints found on node
| grep "$(kubectl get pods --all-namespaces --field-selector spec.nodeName=${NODE_NAME} -o jsonpath='{range .items[*]}{.metadata.name}{" "}{end}' | sed 's/[[:space:]]*$//' | sed 's/[[:space:]]/\\|/g')" /tmp/allpods.out --color=never || echo " No pods found on this node" | ||
| rm /tmp/allpods.out 2>/dev/null || true | ||
| else | ||
| echo " ${YELLOW}Note: kubectl top not available (metrics-server may not be installed)${NC}" |
There was a problem hiding this comment.
Formatting is wrong here and can we have it on a single line?
Resource usage for pods on i-087ae375125081ba5:
\033[0;33mNote: kubectl top not available (metrics-server may not be installed)\033[0m
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR transforms the EKS aperf script into a kubectl plugin and adds several enhancements to improve usability and automation.
Changes
1. Kubectl Plugin Support
eks-aperf.sh→kubectl-aperfto follow kubectl plugin naming conventionskubectl aperf [options]2. Enhanced Script Features
Automatic Taint Handling
kubectl get nodeandjqConfigurable Report Names
--report-nameparameter (default:aperf_record)--report-name="loadtest_run1"Automatic Report Extraction and Browser Opening
index.htmlin the default browser (configurable via--open-browser)open) and Linux (xdg-open)--open-browser=trueImproved Error Handling
kubectl topis unavailable3. Documentation Updates
eks-aperf.shtokubectl-aperfNew Parameters
--report-nameaperf_record--open-browsertrueTesting
Tested on:
✅ macOS (Darwin) with tainted EKS nodes
✅ Automatic browser opening on macOS
✅ Custom report names
✅ Nodes with multiple taints
Breaking Changes
None. The script maintains backward compatibility with all existing parameters.
Future Work
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.